-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
HHH-18325: Prevent comparison between proxied and non-proxied statements #8678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
That sounds like a bug in the Tomcat connection pool, and is something that should be fixed in Tomcat. Thus, I would argue against merging this PR. That said, I am not at all a fan of our
|
Agreed, the original detection of this is due to an issue in Tomcat, which I've raised a separate bug and proposed fix for
The bug in Tomcat has highlighted that Hibernate is discarding objects that it then has to retrieve later on but could be easily passed through the calls to the ResourceRegistry. If the ResourceRegistry can be killed off then this largely becomes irrelevant, but just now Hibernate is presuming that
There's nothing in the JDBC spec that says the same instance has to be returned for a Statement and the calls to a child ResultSet's getStatement method (i.e. Connection Pools or Drivers could create new wrapper/proxy instances for those 2 calls) so an IdentityHashMap still carries a risk of a memory leak. |
As for
Historically that second usage has always been in Hibernate, it was just previously done in a worse way before the introduction of |
I'm fine with the change. I agree with @mc1arke that it, at worst, makes Hibernate more robust. |
OK, fine. I withdraw my objection. |
Some JDBC connection pools create a dynamic reflective Proxy wrapper around connections in the pool, plus the generated statements and the result sets those statements create, with the proxies allowing for methods calls to be intercepted by the connection pool for the purposes of returning items to the pool when they're released rather than destroying the underlying database connection. Tomcat's JDBC pool implementation does not correctly wrap the full chain of objects, meaning the raw Statement can be retrieved from a ResultSet rather than returning the proxied statement. This results in the ResourceRegistry cache attempting to store both the proxied Statement and the non-proxied Statement in its cache, but encountering an exception when the HashMap encounters two entries with the same HashCode and then attempts to differentiate them with an `equals` call which Tomcat's wrapper expects both instances to be proxied connections. To overcome this issue in Tomcat, as well as any other pool implementation which may use Proxy classes but leak the un-proxied entries, the original PreparedStatement used to create the ResultSet is being passed into the GeneratedValuesHelper for passing into the ResourceRegistry, rather than the GeneratedValues helper attempting to extract the Statement from the ResultSet.
Some JDBC connection pools create a dynamic reflective Proxy wrapper around connections in the pool, plus the generated statements and the result sets those statements create, with the proxies allowing for methods calls to be intercepted by the connection pool for the purposes of returning items to the pool when they're released rather than destroying the underlying database connection. Tomcat's JDBC pool implementation does not correctly wrap the full chain of objects, meaning the raw Statement can be retrieved from a ResultSet rather than returning the proxied statement. This results in the ResourceRegistry cache attempting to store both the proxied Statement and the non-proxied Statement in its cache, but encountering an exception when the HashMap encounters two entries with the same HashCode and then attempts to differentiate them with an
equals
call which Tomcat's wrapper expects both instances to be proxied connections. To overcome this issue in Tomcat, as well as any other pool implementation which may use Proxy classes but leak the un-proxied entries, the original PreparedStatement used to create the ResultSet is being passed into the GeneratedValuesHelper for passing into the ResourceRegistry, rather than the GeneratedValues helper attempting to extract the Statement from the ResultSet.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-18325